Skip to content

refactor: ILPP workarounds for 2019.4 and 2020.2+ bugs #447

Merged
0xFA11 merged 8 commits intodevelopfrom
feature/ilpp_refactor
Jan 21, 2021
Merged

refactor: ILPP workarounds for 2019.4 and 2020.2+ bugs #447
0xFA11 merged 8 commits intodevelopfrom
feature/ilpp_refactor

Conversation

@wackoisgod
Copy link
Copy Markdown
Contributor

@wackoisgod wackoisgod commented Jan 18, 2021

IL Postprocessing has some issues in older versions of Unity where we had been running it in process VS 2020.2+ where we run it out of process which prevents some level of assembly conflicts but also has some different issues.

This PR does a couple of things, we hook into the CompilationPipeline and listens for assemblies who have finished compiling and then does some checking and from there runs the ILPostprocessor on the assembly as it had before what makes this work is that the assembly has already been compiled thus we don't run into some of the issues we had in the past.

This API also keeps backwards compatibility with the Unity ILPostprocessor API and can be disabled by setting the USE_UNITY_ILPP scripting define and it will flip back over to using the internal Unity one thus making it easy to keep this for older versions of Unity but go back to the Unity version once issues are resolved.

We also disable RuntimeAccessModifiersILPP which can be re-enabled with MLAPI_RUNTIME_ILPP scripting define. At the moment its no longer needed.

The goal is that we can re-enable the unity version once self-references are fixed and other issues are resolved.

Tested on 2019.4.18f1 and 2020.2.1f1

Should address (MTT-317/316)

@wackoisgod wackoisgod marked this pull request as ready for review January 19, 2021 01:58
using UnityEngine;

#if UNITY_2021_1_OR_NEWER
#warning Built-in Unity ILPP needs to be re-enabled after fixing self-referencing assembly reload issue on 2021.1+ and trunk
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to highlight this and explain here a little further.

As @wackoisgod mentioned in the PR description, we want to re-enable Unity's built-in ILPP instead of this workaround:

The goal is that we can re-enable the unity version once self-references are fixed and other issues are resolved.

However, this workaround is still needed for 2019.4 target but 2020.x(LTS)+ will be OK without the workaround implementation and just work fine with Unity ILPP.
Also to overstate a little, we expect reloading self-referencing assemblies to be fixed on 2021.x+ and trunk in the future, hence this compiler warning on 2021.1+

"dependencies": {
"com.unity.nuget.mono-cecil": "1.10.1-preview.1",
"com.unity.collections": "0.14.0-preview.16"
"com.unity.collections": "0.9.0-preview.6"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Downgrading due to Unity 2019.4 target

Comment on lines +45 to +54
[Browsable(false)]
[EditorBrowsable(EditorBrowsableState.Never)]
[DebuggerBrowsable(DebuggerBrowsableState.Never)]
#if UNITY_2020_2_OR_NEWER && !UNITY_2021_1_OR_NEWER
// RuntimeAccessModifiersILPP will make this `public`
internal static readonly Dictionary<uint, Action<NetworkedBehaviour, BitReader, ulong>> __ntable = new Dictionary<uint, Action<NetworkedBehaviour, BitReader, ulong>>();
#else
[Obsolete("Please do not use, will no longer be exposed in the future versions (framework internal)")]
public static readonly Dictionary<uint, Action<NetworkedBehaviour, BitReader, ulong>> __ntable = new Dictionary<uint, Action<NetworkedBehaviour, BitReader, ulong>>();
#endif
Copy link
Copy Markdown
Contributor

@0xFA11 0xFA11 Jan 19, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hopefully, this "hide and warn" approach would be good enough for users on 2019.4 — they are expected to avoid interacting with these framework internal APIs!

Comment thread com.unity.multiplayer.mlapi/Editor/CodeGen/ILPostProcessorProgram.cs Outdated
Comment thread com.unity.multiplayer.mlapi/Editor/CodeGen/NetworkBehaviourILPP.cs
Copy link
Copy Markdown
Member

@NoelStephensUnity NoelStephensUnity left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other than the minor missing C# standards syntax issues, it looks good!

@0xFA11 0xFA11 merged commit 78b0b9a into develop Jan 21, 2021
@0xFA11 0xFA11 deleted the feature/ilpp_refactor branch March 5, 2021 17:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants